Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add BIN for string function support #2219

Closed
wants to merge 1 commit into from
Closed

Conversation

a6802739
Copy link

@a6802739 a6802739 commented Dec 9, 2016

I have add a function BIN for #310.

cc @siddontang @coocood.

I wonder If I should take negative number into consider, I'm not sure if mysql support negative number now?

@CLAassistant
Copy link

CLAassistant commented Dec 9, 2016

CLA assistant check
All committers have signed the CLA.

@a6802739
Copy link
Author

a6802739 commented Dec 9, 2016

@siddontang , How could I rebase the two commit into one now.

@shenli
Copy link
Member

shenli commented Dec 10, 2016

@a6802739 Thanks for your PR!
You can use git squash command or We can squash your commits when merging your PR.

@shenli
Copy link
Member

shenli commented Dec 10, 2016

@a6802739 You also need to add bin function syntax in parser package. I will send you a document to describe how to do this.

@a6802739
Copy link
Author

@shenli, Thanks for you review. So would you mind send the document to my email [email protected].

if err != nil {
return d, errors.Trace(err)
}
switch {
Copy link
Member

@hanfei1991 hanfei1991 Dec 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to convert an integer to bytes, just use bit operator :)

for(v > 0){
if (v & 1 > 0){
bs = bs + '1'
} else {
bs = bs + '0'
}
v = v >> 1
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, got it.

Copy link
Member

@hanfei1991 hanfei1991 Dec 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BE and LE is about encoding and decoding, needn't be considered here.

Copy link
Author

@a6802739 a6802739 Dec 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var bs []string
for(v > 0){
if (v & 1 > 0){
bs = append(bs, "1")
} else {
bs = append(bs, "0")
}
v = v >> 1
}
i := 0
length := len(bs)
for i < length / 2 {
   temp := bs[i]
   bs[i] = bs[length-i-1]
   bs[length-i-1] = temp
   i = i + 1
}
rs := strings.Join(bs, "")

Or we could use a stack here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appending character '0' and '1' to a byte slice is more efficient than append strings to a string slice.

@coocood
Copy link
Member

coocood commented Dec 12, 2016

@a6802739
I think we better use standard library to implement this function.

fmt.Sprintf("%b", n)

or

strconv.FormatInt(n, 2)

@coocood
Copy link
Member

coocood commented Dec 12, 2016

@a6802739
I tried to run select bin(-1) on MySQL, the result is 1111111111111111111111111111111111111111111111111111111111111111

I seem like a negative integer will be converted to unsigned interger first.

So the stdlib function we should use is strconv.FormatUint.

@a6802739
Copy link
Author

a6802739 commented Dec 12, 2016

@coocood, fmt.Sprintf("%b", n) is good. But it's not suitable for negative integer.

1111111111111111111111111111111111111111111111111111111111111111 is ^(-1) + 1, right?

So, it seems mysql support negative integer.

@coocood
Copy link
Member

coocood commented Dec 12, 2016

@a6802739

x := int64(-1)
fmt.Printf("%b", uint64(x))

@a6802739
Copy link
Author

a6802739 commented Dec 12, 2016

okay. got it.

where should I use the function builtinBIN?

@coocood
Copy link
Member

coocood commented Dec 12, 2016

@a6802739
You can search keyword builtin in PR list, find a PR that adds a new builtin function, see what is needed to add a new function.

@coocood
Copy link
Member

coocood commented Dec 12, 2016

@a6802739
Please make sure test is passed before push a commit.

@ngaut
Copy link
Member

ngaut commented Dec 12, 2016

PTAL

@coocood
Copy link
Member

coocood commented Dec 12, 2016

@a6802739
Copy link
Author

@coocood, where is scanner.l?

@coocood
Copy link
Member

coocood commented Dec 15, 2016

@a6802739
I'm sorry that PR is outdated, checkout this PR instead:
#2249

@shenli
Copy link
Member

shenli commented Dec 19, 2016

@a6802739 Any update?
We write an article to show how to add builtin function: https://github.com/ngaut/builddatabase/blob/master/tidb/builtin.md

@coocood
Copy link
Member

coocood commented Dec 27, 2016

@a6802739
It has been a long time since the last update.
Close for now, if you have any update, I'll reopen it.

@coocood coocood closed this Dec 27, 2016
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants